New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Drop MP3Codec/libmad from paplayer and dvdplayer in favor of FFmpeg #4499
Conversation
Any complaints about reordering the commits so all of the build system stuff is first? That way every commit should compile, which may ease a future bisect. |
Personally I'd prefer that we aim for this to be moved to an add-on rather than completely removed. Ofcourse, the switch to ffmpeg could happen sooner if there's really a bunch of non-compliant files out there (I haven't come across any, but don't use weird encoders), but I'd prefer to leave the support for libmad (ifdef'd out as required) as an incentive for bin add-ons. IIRC @opdenkamp has some work on audio codecs, as well as ofcourse spiff. |
I'm for making a dependency die off. |
There's not really a need to keep the stuff around. We have a perfectly The only benefit I can see to keeping it would be to annoy someone into On Mon, Mar 31, 2014 at 6:04 PM, davilla notifications@github.com wrote:
|
The work has already been done. So it's a matter of annoying folk until it's merged. |
Does ffmpeg need any external lib for this? If yes we would have to add it to our build script and to our mingw environment. |
@t-nelson Project/depend update commits moved to the top. @jmarshallnz Probably there are not many files causing issues, but I have at least one candidate that does a garbling sound at one position, which isn't there when playing back with mpg123/ffmpeg (or even tools like winamp). Also - which honestly is a really "synthetic" case - if you use a tool like MP3Gain to increase the gain of a file and over-amplify to like +20db, libmad will produce really screetchy clipping noise, while anything other will clip. @wsoltys Looking at ffmpeg.git/libavcodec/mpegaudio*, everythings there, but someone with proper ffmpeg knowledge should confirm this ;) |
@wsnipex Could you have a look if our ffmpeg is ready? I tested it with ffmpeg 2.2.x and here everything worked - but not sure if it's build especially cause of some -dev packages I have arround |
@jmarshallnz re audio bin codec add-on: correct, i'll clean it up and create PRs soon. |
configure.in needs cleanup as well. I wonder if we should remove the lame config option and switch cdrip over to ffmpeg as well. @fritsch we use the default mpegaudio codec in ffmpeg2 and it just works. |
Appended some more smaller commits which need squashing:
docs/README* should also be cleaned up (also still mentions libflac stuff which is also gone already), but IMHO that's stuff for another PR. |
FWIW: Tested this on Linux with the FFmpeg 2.2 stuff. Noticed that a few frames are missing at the beginning of every file (even after trying to rewind to 00:00:00), but besides that everything works flawless. Also squashed the additional commits as mentioned above a few days ago. |
why does rewind to 0 not work? would it make any difference if you set the backwards flag to true? |
Yep, that makes a difference (in fact, files are now played back complete without a larger number of missing frames at the beginning) - commit at herrnst@39e323f These SeekTime lines gets pushed to xbmc.log before this (when rewinding to zero, 2940 being the last message, smells alot like the offset):
after (and without missing audio):
Looking at dvdplayer/DVDDemuxers/*.cpp, only the PVR demuxer actually makes use of the flag but that shouldn't be used from within paplayer. FLAC (now also handled by FFmpeg) still seeks fine, can't tell about the other DVDPlayerCodec/FFmpeg formats ATM. EDIT: mentioned commit appended to this PR/branch. |
PR updated: Last two commits make the backwards flag in ::Seek() being set properly (fixing missing frames), and enables accurate seeking in VBR mp3 files, most notable when playing MP3/CUE files. |
…y's SeekTime() in ::Seek when seeking backwards, fixes skipped frames at mp3 file beginnings
Rebased onto master after ffmpeg bump (conflict in DVDDemuxFFmpeg resolved). |
jenkins build this please still angry? (obviously, grrr) |
jenkins build this please |
Hm...
... since jenkins doesn't listen to me, someone mind giving him a push again? |
jenkins build this please |
Drop MP3Codec/libmad from paplayer and dvdplayer in favor of FFmpeg
see: xbmc/xbmc#4499 Signed-off-by: BlackEagle <ike.devolder@gmail.com>
libmad was removed in xbmc#4499 - this one reference erroneously remained.
libmad was removed in xbmc#4499
libmad was removed in xbmc#4499
* VideoPlayer: rtmp fixes * [PVR] Reintroduce filename sort for PVR recordings * [PVR][guiinfo] Extend LISTITEM_HASTIMERSCHEDULE to support pvr timer info tag items. * [Estouchy] update to match the new look of the default skin * [listprovider] fix job cancelling * [estuary] sync * fixed: tighten vcd .dat file checks to avoid unwanted filtering ref http://forum.kodi.tv/showthread.php?tid=298161 * [Estouchy] Song Info Dialog contributors in control list * rbp: Don't close the display here it should be closed by the opener * [DirectoryNodes] override GetChildType() for Episode DirectoryNodes * aml: introduce speaking constants in H264 4K2K check * register is deprecated and does nothing but throw a warning with c++11 * fix initializer list orders * remove extra parantheses to quell compiler warning * fix format specifiers to quell compiler warnings * add default case to quell compiler warnings * add extra braces indicating an initializer list you just won't shut up will you clang? * fixed: wrong signature for IoControl method in PipeFile * fixed: wrong signature for OpenForWrite in RarFile * fixed: wrong signature for method in GUIWindowPrograms * fixed: prototype struct as struct * [lang][kodi.core] automatic syntax corrections for the en_GB language file * [python] allow setting the videoinfotag path * CAddonsDirectory: Improve code clarity in GetScriptsAndPlugins() * CAddonDirectory: Don't clear existing items from item list parameter * DVDCodecUtils: Fix conversion to av_malloc missed by 09acfb8 * BinaryAddonCache: add function to get add-on by ID and type * BinaryAddonCache: optimize Update() function * DVDCodecUtils: Fix buffer overflow if height is odd * StringUtils: template-ize Join() to support more container types * PlayMedia() builtin: Don't clear video playlist if item is not video * [cmake] fix build error when version tag is empty * [cmake] use app name target for application manifest * [win32] fix build when optical is disabled * [win32] fix linking when airtunes or upnp is disabled * [win32] remove unused has audio define * [estuary] sync * bump to v18.0 alpha 1 * [game-settings] Add 'select' as setting type The newly added settings generator generates settings with type 'select'. These were then not loaded correctly. * [addon/depends] Handle autocrlf for depends On Windows, if we have addon dependencies, we need to forcefully disable autocrlf because otherwise the patches will not apply. Patches are typically in LF format (and the build system automatically adds --binary if necessary). But the patches will also not apply if the source files are in CLRF which can happen if the user has core.autocrlf=true in the global git config. A better way of fixing would be to pass the config already to clone: https://gitlab.kitware.com/cmake/cmake/issues/15799 * [addon/depends] Clone only the last commit of dependencies This has been introduced with CMake 3.6 but is ignored on older versions. * use video disptime only in case video is running * FileBrowser: fix multiple item selection * [cmake] Move downloading of patch.exe tool into generic tool script * [cmake] Support @MINGW_TOOLCHAIN_FILE@ in dependency's flags.txt * [binary addons] Support platform specific deps.txt * [binary addons] Add msys and mingw dependencies (from kodi's mingw/msys) * [guilib] Change System.HasModalDialog to System.HasActiveModalDialog and add System.HasVisibleModalDialog * [estuary] system.hasmodaldialog -> system.hasactivemodaldialog * [addon] remove not needed header comment in CScreenSaver The comment on code has confused me a bit and is not really required on so small code. To prevent misunderstand becomes it removed. * fixed: do not use move into member variable here clang points out that it actually hinders copy elusion, not aids it. * [doxygen] add pydocs v18 support * [cleanup][pvr] get rid of obsolete guide views * [cleanup][settings] remove dead code - options filler 'epgguideviews' * [Estouchy] update * Mir windowing system * [retroplayer] Game settings * [retroplayer] Keyboard players * [retroplayer] Game info tags This is mostly a stop-gap implementation until PyRomInfo (https://github.com/garbear/pyrominfo) is included. Thanks to fetzerch for fixing a PVR-related crash. * [retroplayer] IPlayer: Add game flag to player interface * [retroplayer] RetroPlayer core Thanks to elpendor for RGB565 support, poisson for RAII improvements, ChrisMyhre for catching a compile error, notspiff for CMake fixes, acmiyaguchi for video and audio codec support, and popcornmix for Raspberry Pi support (PR 62). TODO: Is a call to `g_renderManager.IsStarted()` needed? * [retroplayer] Game add-ons Thanks to Themaister for rewind functionality, fetzerch for mouse support, file length check and cmake modifications, topfs2 for fixing a crash when loading game clients, eibma for fixing linux compilation errors, a1rwulf for catching a missing callback symbol and fixing some rebase errors, and to notspiff for helping with the rebrand and cmake. * [retroplayer/api] Expose GameInfoTag properties to Python list items * [retroplayer] MyGames window * [PVR] Context menu rewrite & major refactoring of gui actions. * [PVR] Context menu rewrite, gui actions refactoring: adsp settings * [PVR] Context menu rewrite, gui actions refactoring: pvr client menu hook * [PVR] Context menu rewrite, gui actions refactoring: play recording, play channel, resume recording * [PVR] Context menu rewrite, gui actions refactoring: recording info * [PVR] Context menu rewrite: micro opt * [PVR] Context menu rewrite, gui actions refactoring: delete recording * [PVR] Context menu rewrite, gui actions refactoring: rename recording * [PVR] Context menu rewrite, gui actions refactoring: undelete recording * [PVR] Context menu rewrite, gui actions refactoring: mark watched/unwatched * [PVR] Context menu rewrite, gui actions refactoring: rename timer * [PVR] Context menu rewrite, gui actions refactoring: activate / deactivate timer * [PVR] gui actions refactoring: hide channel * [PVR] refactored channels window 'Manage...' context menu item code * [PVR] refactored direct channel number input code (removed logic of derived class from base class) * [PVR] gui actions refactoring: delete all recordings from trash * [PVR] gui actions refactoring: cleanup strings.po * [PVR] Context menu rewrite: Use CStaticContextMenuAction for menu items with static labels. * [PVR] Context menu rewrite, gui actions refactoring: fileitem usage optimizations. * [PVR] refactoring: put all pvr and epg shared_ptr typedefs into one header file instead to typedef again and again in several header files. * [PVR] C++ basics: const vs non-const, reference vs value. :-/ * [settings] move unregister calls into separate methods (similar to register calls) * [settings] move powermanager defaults from CApplication::Create() to CSettings::InitializeDefaults() * [cmake] linux: install game headers * [settings] integrate CSettings into CServiceManager/CServiceBroker * Revert "This makes the SubtitleDialog remember the last service used, and also the last search instead of always using, and searching, the first service in the list." This reverts commit 17525c5. * [aml] windowing: drop EGLNativeTypeAmlAndroid * [aml] amlcodec: use libamcodec.so ... amlplayer on android is no more. * [aml] xbmcapp: drop aml workarounds * [aml] depends: drop libamplayer * [aml] amlutils: android cleanup * Enable VAAPI support for Mir using DRM * [gui] return "" instead of "0" for playlist.position in case nothing is playing * [doxygen] improve coding guidelines for header includes * AML: Make DTS-HD, TrueHD work on IEC61937 backport work (more verbose so we can distinguish) * VideoPlayer: make sure streams are not discarded after a program change * update splash for v18 alpha * [NFS] - in case we resolve a symlink - don't accidently manipulate the original dirent pointer because the memory belongs to libnfs and it will remember the change. Instead use a copy of the dirent instance - fixes strange behavior in some symlink scenarios. thx to ronbaby for making it reproducable * [PVR] Refactor CApplication::PlayMedia to use new PVRGUIActions functionality for playback of PVR channels and recordings. * [OSX/SDL/Windowing] - ignore resize events that exceed the screen resolution (possible bug in SDL related to OSX HiDP scaled resolutions) * [WinSystem/OSX] - ensuer the window origin is reset after resizing - fixes windowed mode appearing on the lower bottom of the screen while ignoring the former origin * [PVR] Fix PVR channel OSD display conditions. * BinaryAddonCache: Fix deadlock Release lock before calling CAddonMgr::IsAddonDisabled(), as this may block in CAddonMgr. * allow absolute item focus * FIX: [droid;dyload] never dyload dependent sytem libs * [epg] Skinning engine: epg grid: add support for alternative (horizontal) grid layout with channels as columns and timeline as rows. * fixed: tag updates on shoutcast streams with low meta data intervals a new tag in came in and updated the position before the previous tag had been processed, meaning the tag never updated after stream start * Fix CID 157138: Control flow issues (DEADCODE) Defect was: AddonsOperations.cpp: 65 in CAddonsOperations::GetAddons(const std::basic_string<char, std::char_traits<char>, std::allocator<char>>&, JSONRPC::ITransportLayer *, JSONRPC::IClient *, const CVariant &, CVariant &)() 59 case ADDON_AUDIO: 60 content = CPluginSource::AUDIO; 61 break; 62 case ADDON_IMAGE: 63 content = CPluginSource::IMAGE; 64 break; >>> CID 157138: Control flow issues (DEADCODE) >>> Execution cannot reach this statement: "case ADDON::ADDON_GAME:". 65 case ADDON_GAME: 66 content = CPluginSource::GAME; 67 break; 68 case ADDON_EXECUTABLE: 69 content = CPluginSource::EXECUTABLE; 70 break; * Fix CID 157137: Error handling issues (CHECKED_RETURN) Defect was: GUIWindowGames.cpp: 258 in GAME::CGUIWindowGames::GetDirectory(const std::basic_string<char, std::char_traits<char>, std::allocator<char>>&, CFileItemList &)() 252 if (!CGUIMediaWindow::GetDirectory(strDirectory, items)) 253 return false; 254 255 // Set label 256 std::string label; 257 if (items.GetLabel().empty()) >>> CID 157137: Error handling issues (CHECKED_RETURN) >>> Calling "IsSource" without checking return value (as is done elsewhere 6 out of 7 times). 258 m_rootDir.IsSource(items.GetPath(), CMediaSourceSettings::GetInstance().GetSources("games"), &label); 259 260 if (!label.empty()) 261 items.SetLabel(label); 262 263 // Set content * Update GameInfoTag This also fixes defects with CID 157143 amd 157144. Defect was: *** CID 157143: Uninitialized members (UNINIT_CTOR) GameInfoTag.h: 35 in GAME::CGameInfoTag::CGameInfoTag()() 29 { 30 class CGameInfoTag : public IArchivable, 31 public ISerializable, 32 public ISortable 33 { 34 public: >>> CID 157143: Uninitialized members (UNINIT_CTOR) >>> Non-static class member "m_year" is not initialized in this constructor nor in any functions that it calls. 35 CGameInfoTag() { Reset(); } 36 CGameInfoTag(const CGameInfoTag& tag) { *this = tag; } 37 const CGameInfoTag& operator=(const CGameInfoTag& tag); 38 ~CGameInfoTag() { } 39 void Reset(); 40 * [estuary] Add missing visible condition * Settings: Enable keyboard players by default This allows the simple 2048 game add-on to be played out-of-the-box. * [cmake] Fix FindXSLT.cmake typo * [PVR] Fix trac xbmc#17108 (duplicate context menu entries for PVR recordings) * MMALRender: Reduce log spam * cec: fixup strings after PR 10775 * [Estouchy] Game support * [estuary] sync * [cmake] fix: command not found "GIT_SHALLOW 1" * We should explicitly check/find libva-drm * CGameLoop: Fix missing destructor * [cmake] Set correct output directory for core_add_shared_library Set the correct output-directory for shared libraries on all platforms (module vs. shared library with different extensions on Darwin) and for all configurations (Debug, Release, RelWithDebInfo, MinSizeRelease). * [cmake/ios] Enable Xcode generator for IOS - Adapt Toolchain file to build a bundles with xcode. This change has to be done in the Toolchain file because CMake otherwise fails to run some compile checks. - Unify path where bundle is built for Makefiles and Xcode to build/$(CONFIGURATION)-$(EFFECTIVE_PLATFORM_NAME) - Set only one architecture to prevent building multi-arch bundles. - Simplify Install.cmake. Existing scrips are called now as post build steps so that the bundle can be executed with Xcode - Apply xcconfig settings. Note: Two adaptions to the existing "autotools/Xcode" scripts were needed. - CMake cannot have the bundle name be different to the binary and we cannot call the binary "Kodi.bin". The script has been adapted to adapt the library execution path directly in place. - CMake calls mkdeb.sh with "Debug-iphoneos", hence the script has been adapted to match strings starting with "debug" or "release". Reference: - https://cmake.org/Bug/view.php?id=15329 - http://stackoverflow.com/questions/33660608/how-can-i-disable-xcode-bitcode-within-a-cmake-project * Fix performace problem with CFileItem::IsGame unnecessarily accessing addon db for PVR content. * [depends] taglib 1.11.1 * [depends] libpng 1.6.26 * [depends] libass 0.13.4 * [depends] curl 7.51.0 * ListItems: Rename "emulator" property to "gameclient" This change apparently got dropped in a RetroPlayer rebase. The reason is because gameclient is more generic than emulator, as the game might be a launchable asset instead of a ROM. Also, I would expect "emulator" to be a human-readable string, e.g. "PCSX Re-armed", whereas "gameclient" is more obviously an add-on ID. * [pvr] remove add-on lib function typedef's * [addon] change namespace from V1::KodiAPI:: to KodiAPI::V1:: * [pvr] bye, bye libXBMC_pvr.cpp * [dependencies] remove remaining references to libmodplug libmodplug was removed in xbmc#9995 so remove remaining references * Remove reference to libxinerama-dev libxinerama-dev is not used so don't document it as a dependency * Remove remaining references to libmad libmad was removed in xbmc#4499 * [cmake] fix building when fribidi was built with glib * VideoPlayer: fix cc after xbmc@03aa244 * [estuary] sync * [gui] fix coverity issue for playlist.position * Remove remaining references to glew glew was (mostly) removed in 03ff0d5; this commit removes any remaining references to it. * [inputstream] bye, bye libKODI_inputstream.cpp * [inputstream] remove add-on lib function typedef's * [addons] remove dead code on CAddonDll * [cmake] Add support for Bluetooth * [cec] missed change in cmake file after cc3ae03 * [lang][skin.estuary] automatic syntax corrections for the en_GB language file * [lang][skin.estuary] updated language files from Transifex * [lang][skin.estouchy] updated language files from Transifex * [cmake] Add support for libcap if we find it
Follow-up to #4386 and initially started by @fritsch - this drops libmad and MP3Codec stuff from paplayer and dvdplayer and rather utilises ffmpeg. While this removes a rather dated dependency (libmad was last updated 2004), the mpg123 decoder inside ffmpeg improves playback of MP3 audio files not being 100% compliant and might play back (partially) distorted when using libmad.
Linux should be fine. OSX (Xcode 5.1) 32bit build tested and working. Win32 project changes need review and testing. Other platforms need to be tested, too.
Probably only for G+1.
Credits belong to @fritsch who started the work :)